Fix ProcessRunner StringBuilder race condition causing assertion failure by Copilot · Pull Request #124269 · dotnet/runtime

Copilot · 2026-02-11T13:11:20Z

Description

ProcessRunner's stdout and stderr handlers concurrently appended to a shared StringBuilder without synchronization, corrupting internal state and triggering assertion failure minBlockCharCount > 0 in ExpandByABlock.

Changes

Lock on _outputCapture StringBuilder in both StandardOutputEventHandler and StandardErrorEventHandler
No lock needed for ToString() call in IsAvailable - process has already exited, no more events will fire

private void StandardOutputEventHandler(object sender, DataReceivedEventArgs eventArgs)
{
    string data = eventArgs?.Data;
    if (!string.IsNullOrEmpty(data))
    {
        WriteLog(data);
        lock (_outputCapture)
        {
            _outputCapture.AppendLine("  " + data);
        }
    }
}

Original prompt

This section details on the original issue you should resolve

<issue_title>Assertion failed: minBlockCharCount > 0</issue_title>
<issue_description>Failed in: runtime-coreclr pgo 20260205.1

Failed tests:

coreclr linux arm Checked defaultpgo @ (Debian.13.Arm32.Open)AzureLinux.3.Arm64.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:debian-13-helix-arm32v7
- readytorun/coreroot_determinism/coreroot_determinism/coreroot_determinism.cmd

Error message:

Process terminated.
Assertion failed.
minBlockCharCount > 0
   at System.Text.StringBuilder.ExpandByABlock(Int32 minBlockCharCount)
   at System.Text.StringBuilder.AppendWithExpansion(Char& value, Int32 valueCount)
   at System.Text.StringBuilder.Append(String value)
   at System.Text.StringBuilder.AppendLine(String value)
   at ProcessRunner.StandardErrorEventHandler(Object sender, DataReceivedEventArgs eventArgs) in /_/src/coreclr/tools/r2rtest/ProcessRunner.cs:line 297
   at System.Diagnostics.Process.ErrorReadNotifyUser(String data) in /_/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs:line 1731
   at System.Diagnostics.AsyncStreamReader.FlushMessageQueue(Boolean rethrowInNewThread) in /_/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/AsyncStreamReader.cs:line 237
   at System.Diagnostics.AsyncStreamReader.ReadBufferAsync() in /_/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/AsyncStreamReader.cs:line 123
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.ExecutionContextCallback(Object s)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.MoveNext(Thread threadPoolThread)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.MoveNext()
   at System.Threading.Tasks.AwaitTaskContinuation.RunOrScheduleAction(IAsyncStateMachineBox box, Boolean allowInlining)
   at System.Threading.Tasks.Task.RunContinuations(Object continuationObject)
   at System.Threading.Tasks.Task`1.TrySetResult(TResult result)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.SetExistingTaskResult(Task`1 task, TResult result)
   at System.Runtime.CompilerServices.AsyncValueTaskMethodBuilder`1.SetResult(TResult result)
   at System.IO.Pipes.PipeStream.ReadAsyncCore(Memory`1 destination, CancellationToken cancellationToken) in /_/src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Unix.cs:line 313
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.ExecutionContextCallback(Object s)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.MoveNext(Thread threadPoolThread)
   at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.MoveNext()
   at System.Threading.ThreadPool.<>c.<.cctor>b__47_0(Object state)
   at System.Threading.Tasks.Sources.ManualResetValueTaskSourceCore`1.SignalCompletion()
   at System.Threading.Tasks.Sources.ManualResetValueTaskSourceCore`1.SetResult(TResult result)
   at System.Net.Sockets.Socket.AwaitableSocketAsyncEventArgs.OnCompleted(SocketAsyncEventArgs _) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs:line 1013
   at System.Net.Sockets.SocketAsyncEventArgs.OnCompletedInternal() in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs:line 214
   at System.Net.Sockets.SocketAsyncEventArgs.FinishOperationAsyncSuccess(Int32 bytesTransferred, SocketFlags flags) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs:line 1130
   at System.Net.Sockets.SocketAsyncEventArgs.CompletionCallback(Int32 bytesTransferred, SocketFlags flags, SocketError socketError) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Unix.cs:line 389
   at System.Net.Sockets.SocketAsyncEventArgs.TransferCompletionCallbackCore(Int32 bytesTransferred, Memory`1 socketAddress, SocketFlags receivedFlags, SocketError socketError) in /_/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Unix.cs:line 108
   at System.Net.Sockets.SocketAsyncContext.BufferMemoryReceiveOperation.InvokeCallback(Boolean all

Stack trace:

   at Xunit.Assert.True(Nullable`1 condition, String userMessage) in /_/src/arcade/src/Microsoft.DotNet.XUnitAssert/src/BooleanAsserts.cs:line 141
   at Xunit.Assert.True(Boolean condition, String userMessage) in /_/src/arcade/src/Microsoft.DotNet.XUnitAssert/src/BooleanAsserts.cs:line 123
   at TestLibrary.OutOfProcessTest.RunOutOfProcessTest(String assemblyPath, String ...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes dotnet/runtime#124067

<!-- START COPILOT CODING AGENT TIPS -->
---

✨ Let Copilot coding agent [set things up for you](https://github.com/dotnet/runtime/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: MihaZupan <25307628+MihaZupan@users.noreply.github.com>

stephentoub · 2026-02-11T13:31:25Z

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

Copilot

Pull request overview

This PR addresses a race condition in ProcessRunner’s stdout/stderr capture that can corrupt the shared StringBuilder and trigger StringBuilder.ExpandByABlock assertions during concurrent appends.

Changes:

Adds synchronization around _outputCapture.AppendLine(...) in both stdout and stderr event handlers.
Switches synchronization to lock on the _outputCapture instance itself (instead of a separate lock object).

src/coreclr/tools/r2rtest/ProcessRunner.cs

Copilot · 2026-02-11T13:36:20Z

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

🤖 Copilot Code Review — PR #124225

Holistic Assessment

Motivation: The bug is real — StandardOutputEventHandler and StandardErrorEventHandler are invoked on separate thread pool threads (via AsyncStreamReader) and both append to the shared _outputCapture StringBuilder without synchronization, causing internal state corruption. The linked CI failure confirms this.

Approach: Adding lock around the concurrent AppendLine calls is the minimal correct fix. Locking on the _outputCapture instance itself (a private readonly field) is acceptable and consistent with the existing pattern in this file where _logWriter is also used as its own lock object (lines 240, 270, 376). Not locking the ToString() call in IsAvailable is correct — by that point, the process has exited and WaitForExit(0) has been called (line 324), so stdout/stderr streams have been fully drained and no more events will fire.

Summary: ✅ LGTM. The fix is minimal, correct, and consistent with the existing file conventions.

Detailed Findings

✅ Thread-safety fix is correct and sufficient

The two event handlers (StandardOutputEventHandler at line 281, StandardErrorEventHandler at line 294) are fired from AsyncStreamReader.ReadBufferAsync on thread pool threads — one for stdout, one for stderr — so they can run concurrently. The lock correctly serializes access to the shared _outputCapture StringBuilder, preventing the internal state corruption that caused the minBlockCharCount > 0 assertion failure.

✅ No lock needed on `ToString()` in `IsAvailable`

The _outputCapture.ToString() call (line 369) occurs after _process.WaitForExit(0) (line 324) returns. By that point, the async stream readers have completed and no more DataReceived events will fire, so there is no concurrent access to guard against. This is consistent with the Process contract where WaitForExit ensures all async output has been flushed.

✅ Locking on `_outputCapture` itself is consistent with existing file patterns

The file already uses the _logWriter object as its own lock (lines 240, 270, 376). Using _outputCapture as the lock object follows the same convention and avoids an unnecessary extra allocation. The field is private readonly so there's no risk of external code locking on the same instance.

MihaZupan · 2026-02-11T23:37:59Z

/ba-g DeadLetter on Apple TV, the rest are known

Initial plan

a93f698

Copilot AI assigned Copilot and MihaZupan Feb 11, 2026

Copilot started work on behalf of MihaZupan February 11, 2026 13:11 View session

Fix thread-safety issue in ProcessRunner StringBuilder access

6be2b02

Co-authored-by: MihaZupan <25307628+MihaZupan@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix assertion failure in StringBuilder expansion~~ Fix race condition in ProcessRunner causing StringBuilder assertion failure Feb 11, 2026

Copilot AI requested a review from MihaZupan February 11, 2026 13:17

Copilot finished work on behalf of MihaZupan February 11, 2026 13:17

Copilot started work on behalf of MihaZupan February 11, 2026 13:18 View session

github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 11, 2026

Lock on StringBuilder itself and remove lock from ToString

df031c7

Co-authored-by: MihaZupan <25307628+MihaZupan@users.noreply.github.com>

MihaZupan marked this pull request as ready for review February 11, 2026 13:23

Copilot AI review requested due to automatic review settings February 11, 2026 13:23

MihaZupan approved these changes Feb 11, 2026

View reviewed changes

Copilot AI changed the title ~~Fix race condition in ProcessRunner causing StringBuilder assertion failure~~ Fix ProcessRunner StringBuilder race condition causing assertion failure Feb 11, 2026

Copilot AI requested a review from MihaZupan February 11, 2026 13:23

Copilot finished work on behalf of MihaZupan February 11, 2026 13:23

MihaZupan requested a review from EgorBo February 11, 2026 13:23

Copilot started reviewing on behalf of MihaZupan February 11, 2026 13:24 View session

Copilot started work on behalf of stephentoub February 11, 2026 13:31 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

src/coreclr/tools/r2rtest/ProcessRunner.cs Show resolved Hide resolved

Copilot AI requested a review from stephentoub February 11, 2026 13:37

Copilot finished work on behalf of stephentoub February 11, 2026 13:37

stephentoub approved these changes Feb 11, 2026

View reviewed changes

MihaZupan enabled auto-merge (squash) February 11, 2026 14:45

build-analysis bot mentioned this pull request Feb 11, 2026

System.Security.Cryptography.CryptographicException : m_safeCertContext is an invalid handle. #124279

Closed

MihaZupan merged commit 3120fe3 into main Feb 11, 2026
103 of 112 checks passed

MihaZupan deleted the copilot/fix-assertion-failure branch February 11, 2026 23:38

dotnet-maestro bot mentioned this pull request Feb 12, 2026

[main] Source code updates from dotnet/runtime dotnet/dotnet#4810

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix ProcessRunner StringBuilder race condition causing assertion failure#124269

Fix ProcessRunner StringBuilder race condition causing assertion failure#124269
MihaZupan merged 3 commits intomainfrom
copilot/fix-assertion-failure

Copilot AI commented Feb 11, 2026 •

edited by MihaZupan

Loading

Uh oh!

stephentoub commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI commented Feb 11, 2026

Uh oh!

MihaZupan commented Feb 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented Feb 11, 2026 • edited by MihaZupan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Uh oh!

stephentoub commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI commented Feb 11, 2026

🤖 Copilot Code Review — PR #124225

Holistic Assessment

Detailed Findings

✅ Thread-safety fix is correct and sufficient

✅ No lock needed on ToString() in IsAvailable

✅ Locking on _outputCapture itself is consistent with existing file patterns

Uh oh!

MihaZupan commented Feb 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Feb 11, 2026 •

edited by MihaZupan

Loading

✅ No lock needed on `ToString()` in `IsAvailable`

✅ Locking on `_outputCapture` itself is consistent with existing file patterns